CodSpeed Performance Report

Merging #1831 will not alter performance

_{Comparing alleyshairu:passthrough_nodejs_and_python_error_message_fix (81ac320) with main (8dffbdf)}

Summary

✅ 38 untouched benchmarks

The implementation

I agree that for errors exposed by a library a properly defined error enum is very important.
However, for this kind of adhoc application level error, I find that we are better off with the flexibility of anyhow.

I think that this error would actually benefit from using anyhow.
Currently we get the error:

called `Result::unwrap()` on an `Err` value: Custom { kind: NotFound, error: "Attempted to run the command `npm install` but npm does not exist. Have you installed nodejs?" }

However by swapping to anyhow we can make this a little cleaner.

called `Result::unwrap()` on an `Err` value: Attempted to run the command `npm install` but npm does not exist. Have you installed nodejs?

The test

Personally I wouldnt have written a test here, since the functionality is improving error messages in the test suite, not the application itself.
Regardless, I'm quite happy to accept one, it will still bring some value.

I see no issue with comparing strings in the test here, since what we want to test is that the error message is written in a specific way.

In fact the one issue with the test is that we arent properly comparing the strings.
Calling the logic under test twice and comparing the results isnt really testing anything at all.
Instead of calling command_not_found_error_message in the test, we should provide the specific string

Attempted to run the command `shotover_non_existent_command arg1 arg2` but shotover_non_existent_command does not exist. Have you installed shotover_non_existent_command?

and compare against that.

In general though, your PR is looking good.